Skip to content

fix(Listbox): prevent virtualized list from scrolling page on mount#2675

Open
benjamincanac wants to merge 1 commit into
unovue:v2from
benjamincanac:fix/listbox-virtual-mount-scroll
Open

fix(Listbox): prevent virtualized list from scrolling page on mount#2675
benjamincanac wants to merge 1 commit into
unovue:v2from
benjamincanac:fix/listbox-virtual-mount-scroll

Conversation

@benjamincanac
Copy link
Copy Markdown
Contributor

@benjamincanac benjamincanac commented Jun 4, 2026

Follow-up to #2666, which fixed the non-virtual path only. A virtualized Listbox still pulled the whole page to itself on mount (same root cause as nuxt/ui#6513).

Root cause

In ListboxRoot, highlightSelected(event, scroll) dropped the scroll flag whenever isVirtual was true — it just called virtualFocusHook.trigger(event). So on mount a virtualized list with no checked item (e.g. multiple + empty v-model) fell into the hook's elsehighlightFirstItem() → synthetic PageUp → changeHighlight(firstItem) with default focus + scrollIntoView, yanking a below-the-fold listbox into view on load.

Fix

Thread the mount intent through the virtualFocusHook payload ({ event, scroll }) so the virtualizer sets its roving-tabindex target without focusing/scrolling on mount, mirroring the non-virtual path. The checked-item branch keeps scrollToIndex — that only scrolls the internal listbox container, never the page.

Notes for reviewers

  • Behavior change (intended): aligns virtualized mount behavior with the non-virtual fix from fix(Listbox): prevent page scroll on mount caused by changeHighlight #2666. User-driven paths (keyboard, typeahead, select, entry-focus) are unchanged.
  • Internal type change: virtualFocusHook's payload shape changed. It lives on the context exposed by injectListboxRootContext, but it's an internal coordination hook not realistically triggered/consumed externally — no public API or runtime behavior for normal usage changes. Not a breaking change.
  • Tests cover both mount cases (no selection → first item; pre-selected → checked item), each asserting no page scroll + no focus theft; plus a regression guard that user entry-focus does focus/scroll.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Virtual listboxes now properly manage focus and scrolling during initial mount and item highlighting.
    • Eliminated unwanted focus stealing and document-level scrolling on component initialization.
    • Pre-selected items now correctly highlight without triggering unintended focus changes or scrolling.
  • Tests

    • Added comprehensive test coverage for virtualized listbox focus and scroll behavior in various user interaction scenarios.

Follow-up to unovue#2666, which fixed the non-virtual path only. A virtualized
Listbox still pulled the whole page to itself on mount: `highlightSelected`
dropped the `scroll` flag whenever `isVirtual` was true, so a list with no
checked item fell through to `highlightFirstItem` (synthetic PageUp →
`changeHighlight` with focus + `scrollIntoView`), yanking a below-the-fold
listbox into view on load.

Thread the mount intent through the `virtualFocusHook` payload
(`{ event, scroll }`) so the virtualizer can set its roving-tabindex target
without focusing/scrolling on mount, mirroring the non-virtual path. The
checked-item branch keeps `scrollToIndex`, which only scrolls the internal
listbox container, never the page.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59142963-5883-4323-bce8-9cd1436f176f

📥 Commits

Reviewing files that changed from the base of the PR and between 091d650 and 9a5507a.

📒 Files selected for processing (3)
  • packages/core/src/Listbox/Listbox.test.ts
  • packages/core/src/Listbox/ListboxRoot.vue
  • packages/core/src/Listbox/ListboxVirtualizer.vue

📝 Walkthrough

Walkthrough

This PR refactors the virtual focus hook contract in the Listbox component to distinguish between initial mount highlighting and user-driven scrolling. The virtualFocusHook now receives { event, scroll } instead of a single event, enabling different behavior: mount scenarios skip focus and scrolling, while user interactions apply them.

Changes

Virtual focus hook contract and scroll-aware implementation

Layer / File(s) Summary
Virtual focus hook type contract
packages/core/src/Listbox/ListboxRoot.vue
virtualFocusHook is retyped from accepting Event | null | undefined to { event?: Event, scroll: boolean } in ListboxRootContext and at instantiation.
Hook invocation from ListboxRoot
packages/core/src/Listbox/ListboxRoot.vue
The virtualFocusHook.trigger call now passes { event, scroll }, providing both the event and a scroll flag during programmatic highlight handling.
Virtual focus handler and scroll-aware logic
packages/core/src/Listbox/ListboxVirtualizer.vue
The handler destructures { event, scroll } and branches: scroll-driven updates scroll the virtualizer and highlight with focus; mount scenarios (scroll=false) highlight without scroll or focus side effects.
Virtualized listbox test coverage
packages/core/src/Listbox/Listbox.test.ts
Imports ListboxVirtualizer, stubs JSDOM browser APIs, defines a VirtualListbox component rendering 100 options, and verifies mount highlight does not steal focus/scroll, pre-selected items highlight correctly, and user-driven focus scrolls and focuses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • unovue/reka-ui#2666: Both PRs update Listbox mount-time highlighting to prevent unwanted focus/scroll side effects via similar mechanisms in virtual and non-virtual paths.
  • unovue/reka-ui#2607: Both PRs modify ListboxRoot.vue highlight/focus code paths, one retyping the virtual focus hook payload and the other adding SSR guards to skip DOM side effects.
  • unovue/reka-ui#2651: Both PRs address incorrect highlight state on initial mount by adjusting how virtual focus and highlight logic distinguish between mount and interactive scenarios.

Poem

🐰 A listbox springs to life, but must not leap
Mount-time highlights sneak in quiet, while scrolls and focus keep
Their energy for clicks and keys, when users ask to see.
With { event, scroll } in hand, the virtualizer runs carefree! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preventing unwanted page scrolling when a virtualized Listbox mounts. It directly addresses the primary bug fix across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 4, 2026

Open in StackBlitz

npm i https://pkg.pr.new/reka-ui@2675

commit: 9a5507a

@benjamincanac benjamincanac marked this pull request as ready for review June 5, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant